Skip to content

feat(gfi): migrate plugin - #447

Open
oeninghe-dataport wants to merge 146 commits into
nextfrom
vue3/migrate-plugin-gfi
Open

feat(gfi): migrate plugin#447
oeninghe-dataport wants to merge 146 commits into
nextfrom
vue3/migrate-plugin-gfi

Conversation

@oeninghe-dataport

Copy link
Copy Markdown
Collaborator

Summary

Migrate the GFI plugin.

Instructions for local reproduction and review

  • Open snowbox.
  • Click on a marker.
  • See the result.

Additional hints

  • The utils requestGfi* were migrated as-is and do not need to be reviewed therefore.

Relevant tickets, issues, et cetera

Closes #368

@oeninghe-dataport oeninghe-dataport added this to the POLAR@3 milestone Jan 8, 2026
@oeninghe-dataport oeninghe-dataport self-assigned this Jan 8, 2026
@oeninghe-dataport oeninghe-dataport added the refactor Refactoring of previous code label Jan 8, 2026
@oeninghe-dataport oeninghe-dataport linked an issue Jan 12, 2026 that may be closed by this pull request
@oeninghe-dataport
oeninghe-dataport force-pushed the vue3/migrate-plugin-gfi branch 3 times, most recently from 58e73f6 to f941649 Compare January 16, 2026 13:12
@github-actions

github-actions Bot commented Jan 23, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://Dataport.github.io/polar/pr-preview/pr-447/

Built to branch gh-pages at 2026-07-28 13:11 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@oeninghe-dataport
oeninghe-dataport marked this pull request as ready for review February 17, 2026 16:02
@dopenguin
dopenguin removed the request for review from warm-coolguy February 19, 2026 09:46
# Conflicts:
#	examples/snowbox/services.js

@dopenguin dopenguin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Please add an example to iceberg; this maybe should include an example with a layer where the feature list is not being used
  • There should be no horizontal scrollbar if no features are available Image
  • Some things are missing / quite different with the featureList; some parts are connections with the markers feature. This includes:
    • When hovering an element in the feature list, the feature is highlighted in the map with the hover style
    • When hovering an element in the map, the feature is highlighted in the feature list (previously green); when hovering a clustered feature, all features that are part of the cluster are highlighted
    • If I select a feature in the map, it is selected in the feature list
    • If I select a feature in the feature list, the corresponding marker gets the selected style; currently, a yellow dot is being displayed
    • If I select a feature in the feature list, the map should be centered on that feature
    • If a feature is not selectable because of the configured isSelectable function, it is not being shown in the feature list

The list may not be complete, so please take a look at Meldemichel regarding the various things mentioned above.

I'll be taking a look at the components and stores once you've tackled these things.

🏓 @oeninghe-dataport

Comment thread src/plugins/gfi/utils/requestGfiWfs.ts Outdated
Comment thread src/plugins/gfi/utils/requestGfiWfs.ts
Comment thread src/plugins/gfi/utils/requestGfiWms.ts Outdated
Comment thread src/plugins/gfi/utils/requestGfiWms.ts Outdated
Comment thread src/core/stores/main.ts
Comment thread src/plugins/geoLocation/types.ts
Comment thread examples/snowbox/index.js Outdated
Comment thread src/locales.ts Outdated
Comment thread src/locales.ts Outdated
Comment thread src/locales.ts Outdated
dopenguin and others added 5 commits February 23, 2026 19:56
Co-authored-by: Pascal Röhling <73653210+dopenguin@users.noreply.github.com>
Co-authored-by: Pascal Röhling <73653210+dopenguin@users.noreply.github.com>
Co-authored-by: Pascal Röhling <73653210+dopenguin@users.noreply.github.com>
Co-authored-by: Pascal Röhling <73653210+dopenguin@users.noreply.github.com>
@oeninghe-dataport

Copy link
Copy Markdown
Collaborator Author

🏓 @dopenguin

Every point was (hopefully) addressed again, including the aspects discussed in person.

Comment thread examples/snowbox/index.html Outdated
@dopenguin

dopenguin commented Aug 3, 2026

Copy link
Copy Markdown
Member

The amount of displayed features seems to be off by a lot compared to https://static.hamburg.de/kartenclient/prod/.
This would also be interesting to test in https://github.com/Dataport/polar/pull/447/changes#r3364630910

Current implementation:
Bildschirmfoto 2026-08-03 um 19 37 11

Meldemichel (prod):
Bildschirmfoto 2026-08-03 um 19 37 57

@@ -0,0 +1,168 @@
<template>
<p class="kern-subline kern-subline--small">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This element should not have any padding if the nav is not rendered.

}
}

.ol-overlay-container.ol-selectable {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment for which situation this is required.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/core/stores/index.ts Outdated
Comment thread src/core/index.ts
export * from './utils/export/plugin'
export * from './utils/export/store'

export { isVisible } from '@/lib/invisibleStyle'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why should a lib-function be re-exported from the core?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is used at https://github.com/Dataport/polar/blob/vue3/migrate-plugin-gfi/examples/snowbox/index.js#L454

As there is a usage in our example client, there is surely also a valid usage for real-world clients.

Comment on lines +45 to +47
{{
gfiStore.title
}}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the linter autoformat that to three lines?


.action-bar-group {
display: flex;
gap: var(--kern-metric-space-default);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
gap: var(--kern-metric-space-default);
gap: var(--kern-metric-space-small);

Inline with the +- and --buttons in routing.

value: 'Wert',
},
property: {
export: 'Export als PDF',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not really seem to fit as exportProperty does not declare what kind of export is present.

</KernButton>
<KernButton
class="kern-btn--tertiary"
icon="kern-icon--close"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, the icon and the ariaLabel depended whether featureList is used or not. This should stay like this.

class="kern-btn--tertiary"
icon="kern-icon--close"
:label-sr-only="true"
@click="gfiStore.selectedFeatures = markRaw({})"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

markRaw is unnecessary for shallowRefs. This can be removed here and in GfiFeatureList.ce.vue and list.ts

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also remove the now "unused" imports

Comment thread src/lib/getCluster.ts
Comment on lines -28 to +55
const cluster = (layer.getSource() as VectorSource)
.getFeatures()
.find((candidate: Feature) => candidate.get('features').includes(feature))
const source = layer.getSource() as VectorSource
const cachedCluster = clusterByFeature.get(feature)
const cluster =
cachedCluster &&
source.hasFeature(cachedCluster) &&
cachedCluster.get('features').includes(feature)
? cachedCluster
: source
.getFeatures()
.find((candidate: Feature) =>
candidate.get('features').includes(feature)
)

if (!(cluster instanceof Feature)) {
throw new Error(
'@polar/lib-get-cluster: No cluster could be found for the given feature.'
)
}
clusterByFeature.set(feature, cluster)
// The given feature should be the last in the array, as it the one "above" all thus added last
cluster.set('features', [
...cluster.get('features').filter((f: Feature) => f !== feature),
feature,
])
const clusterFeatures = cluster.get('features') as Feature[]
if (clusterFeatures.at(-1) !== feature) {
cluster.set(
'features',
[...clusterFeatures.filter((f) => f !== feature), feature],
true
)
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been working properly since January 2024 and I don't see the new solution to be easier to read.
Why is this change necessary?

oeninghe-dataport and others added 2 commits August 4, 2026 14:53

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a patch that
a) changes kern-heading-medium to kern-title like https://www.kern-ux.de/komponenten/card intends
b) restructures the CSS

Comment on lines +125 to +130
background-color: #dff0dd;
}

&:hover {
border-radius: var(--kern-metric-border-radius-default);
border-color: #3fa535;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both have to use the oklch equivalent

Co-authored-by: Pascal Röhling <73653210+dopenguin@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Refactoring of previous code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migration of GFI plugin to POLAR@3

2 participants